-
Notifications
You must be signed in to change notification settings - Fork 609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding support for directories/packagePath in package.json. #8
Conversation
@@ -8,7 +8,7 @@ export interface IExecutable { | |||
name?: string; | |||
|
|||
/** Optional callback to indicate if the task is enabled or not. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For props like this which takes in callbacks, good to document what kind of callback signature is expected..
export interface IBuildReceiptTask { | ||
} | ||
|
||
const PROCESS_OUTPUT_DELIMITER: string = '~X~X~X~X~X~X~'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good to use in the delimiter here, symbols which are disallowed in file paths
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, consecutive slashes ( X///)X%X\X
/** | ||
* This task is responsible for the following: | ||
* | ||
* 1. Gather the local files build receipt of the current files. Cache it in _lastFilesHash for later use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Gather the local files build receipt of...", "local files build receipt" is a little hard to parse...
* 2. Check for the existence of a package build receipt. If none exists, complete the task. | ||
* | ||
* 3. Compare the package build hashes with the local build hashes. If they match, update the buildConfig to | ||
* have isRedundantBuild flat set to true. This allows other tasks to completely prematurely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworded comments.
@@ -0,0 +1,46 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err, what are these? Do you actually intend to check these in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we need a separate project in the mix that actually tests that the rig works.
|
||
getLocalHashes().then(localHashes => { | ||
_lastLocalHashes = localHashes; | ||
debugger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debugger statement here
this.buildConfig.isRedundantBuild = true; | ||
this.log('Build is redundant. Skipping steps.'); | ||
} else { | ||
debugger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
…into dzearing/package-support # Conflicts: # common/npm-shrinkwrap.json # gulp-core-build/src/index.ts
…list Don't hook into the process exit when they run gulp --tasks
* Updating core build to respect directories/packagePath in package.json. * Removing web-library-build integration for now. * PR feedback addressed. * Allowing karma and webpack to be skipped correctly and consistently.
…ariable Allowing setWebpackPublicPath to use a context variable for the script name regex.
Update @rushstack/heft-sass-plugin to be compatible with multi-phase Heft
This change allows projects to specify they build their package output in a specific directory. This allows us to make major optimizations in npmx workflows, like unpacking the packages to prepopulate the build output.
It also includes a prototype build receipt task, which is actually 2 tasks: a check step and an update step. The check step evaluates if files in the project have changed since last built. It does this by looking at git sha1 hashes for files and comparing them hashes in a build.json file published in the package folder. If the hashes match, the remaining subtasks are skipped. If they dont, the rest of the subtasks run and the update step runs last, which updates the build.json with the latest hashes.
That means that if package files haven't changed since last published, we can avoid building completely.
I'd like to also include the change detection tasks, but won't modify the build rigs to include it until we've sorted out the right layer to include it. Per discussion, right now the git hash logic takes 50-100ms for a project. We're looking at moving this logic to a higher level within NPMX, so that we can further optimize completely avoiding spinning up processes to load gulp and all that stuff. Right now just to load up gulp in a quadcore i7 macbook pro, it takes 3 seconds. Multiplied times 45 packages, we end up spending 135 seconds just loading gulp on a pure warm build. So if we can reduce that down to 50-100ms per package, we're looking at slimming this down to maybe 4 seconds of overhead.